Skip to content

Escape user input in publish review to prevent XSS#43

Merged
TeoSlayer merged 1 commit into
mainfrom
fix/publish-review-xss
Jun 22, 2026
Merged

Escape user input in publish review to prevent XSS#43
TeoSlayer merged 1 commit into
mainfrom
fix/publish-review-xss

Conversation

@TeoSlayer

Copy link
Copy Markdown
Contributor

Vulnerability

renderReview() in src/pages/publish.astro built the review table by interpolating form-state values directly into innerHTML. Only the row labels (k) were passed through esc(); the user-controlled values (v) — App ID, description, backend URL, header values, method names/paths, vendor name/URL, capabilities, signer name, etc. — were inserted raw.

Any value containing markup (e.g. <img src=x onerror=...>) executes when the publisher reaches the review step — a self-XSS on the publish flow.

Fix

Escape every user-controlled value with the existing esc() helper at row-construction time, while preserving the intentional markup:

  • The Methods row keeps its <br> separators, but each method string is escaped individually before joining.
  • The empty-field em-dash placeholder is unchanged: esc('') is falsy, so the shared v||'<span…>—</span>' fallback still renders the placeholder.
  • Non-user literals (HTTP API, the · vendor separator) are unaffected; visual output is identical.

Verification

  • npm run build (astro build) — 190 pages built, publish.astro compiles cleanly, no breakage.

@github-actions

Copy link
Copy Markdown

🚀 Preview deployed to Cloudflare Pages

  • Commit deploy URL: https://f473c82f.pilotprotocol.pages.dev
  • Branch alias: https://fix/publish-review-xss.pilotprotocol.pages.dev (may take ~30s to propagate)
  • Commit: fc90a71787edaf2e00531366534e72f3e7882a9d

@TeoSlayer TeoSlayer merged commit 4adea52 into main Jun 22, 2026
2 checks passed
@matthew-pilot matthew-pilot deleted the fix/publish-review-xss branch June 22, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants